Complete #1560 by adding deconstructing foreach#1689
Conversation
Closes dotnet#1602 - Complete dotnet#1560 by covering *foreach_statement* using a *deconstructor* - Add sample “Deconstructing foreach” - Change subclause title from “await foreach” to “Asynchronous foreach” for consistency with siblings - Also cleans up some whitespace
BillWagner
left a comment
BillWagner
left a comment
There was a problem hiding this comment.
Great start. It looks like the specter of discards in a deconstructing assignment has made its appearance again.
| try | ||
| { | ||
| while (await enumerator.MoveNextAsync()) | ||
| var enumerator = enumerable.GetAsyncEnumerator(); |
There was a problem hiding this comment.
It looks like the only change here is indenting. Correct?
There was a problem hiding this comment.
The expansions for the others where all wrapped in an enclosing block and this one was missed/different. So yes, the whole thing was indented and unindented { & } added to wrap it – not that the diff makes that easy to spot!
|
|
||
| #### §deconstructing-foreach Deconstructing foreach | ||
|
|
||
| A deconstructing foreach replaces the declaration and initialisation of a single iteration variable per iteration, in the synchronous and asynchronous foreach statements, with a collection of zero or more iteration variables declared per iteration within the *deconstructor*([§12.23.3](expressions.md#12233-deconstructing-assignment)) of a *deconstructing_assignment*. |
There was a problem hiding this comment.
Similar to the changes in 1560, this should be "2 or more":
| A deconstructing foreach replaces the declaration and initialisation of a single iteration variable per iteration, in the synchronous and asynchronous foreach statements, with a collection of zero or more iteration variables declared per iteration within the *deconstructor*([§12.23.3](expressions.md#12233-deconstructing-assignment)) of a *deconstructing_assignment*. | |
| A deconstructing foreach replaces the declaration and initialisation of a single iteration variable per iteration, in the synchronous and asynchronous foreach statements, with a collection of two or more iteration variables declared per iteration within the *deconstructor*([§12.23.3](expressions.md#12233-deconstructing-assignment)) of a *deconstructing_assignment*. |
There was a problem hiding this comment.
@BillWagner – I had completely forgotten that #1639 introduced targets and counted them rather than assigned named variables, so I just wrote this doing the same as that is what is important – discards are discarded afterall :-)
Your proposed change here however makes it “two or more iteration variables” so more wordsmithing would be required to go the “target” route. I think targets would not fit well here, but then I went with the consensus on §12.23 so that is not too surprising. [Note that #1639 actually contradicts itself, it starts by stating two or more targets are assigned, then later that discard targets are not assigned (“but no value is stored”).]
That discards may be present is covered just below in line 1444. I did consider whether to list the allowed, which would have surfaced discards, or disallowed alternatives, and decided the key issue was that existing variables could not be assigned to so highlighted that by just listed the disallowed variable_reference.
Suggestion: We leave this PR as is on this topic and then you & @jskeet raise an issue to align the text in §12.23 to either use targets (which are not all assigned) in both, neither, or go with variety is the spice of life :-) Mark it pre-C#9 so its not a blocker – what exists here & in §12.23 are not wrong per se.
There was a problem hiding this comment.
That suggestion is fine by me.
| } | ||
| ``` | ||
|
|
||
| this follows the behavior of synchronous foreach ([§13.9.5.2](statements.md#13952-synchronous-foreach)), differing by replacing the delaration and initialisation of a single iteration variable with a *deconstructing_assignment* which declares and assigns zero or more initialisation variables: |
There was a problem hiding this comment.
Again, 2 or more, and we should add text about how discards are handled:
| this follows the behavior of synchronous foreach ([§13.9.5.2](statements.md#13952-synchronous-foreach)), differing by replacing the delaration and initialisation of a single iteration variable with a *deconstructing_assignment* which declares and assigns zero or more initialisation variables: | |
| this follows the behavior of synchronous foreach ([§13.9.5.2](statements.md#13952-synchronous-foreach)), differing by replacing the delaration and initialisation of a single iteration variable with a *deconstructing_assignment* which declares and assigns two or more initialisation variables: |
There was a problem hiding this comment.
See comment on line 1442.
|
|
||
| > *Note*: An `await foreach` is not required to dispose of `e` synchronously if an asynchronous dispose mechanism is not available. *end note* | ||
|
|
||
| #### §deconstructing-foreach Deconstructing foreach |
There was a problem hiding this comment.
I ran a couple tests, and we need to specify discards in a deconstructing foreach. I think the language should be similar to what was added in #1639.
What's interesting here is that a synchronous and asynchronous foreach don't allow the iteration variable to be a discard. However, in a deconstructing foreach, any number of the iteration variables may be a discard (from 0 to all).
There was a problem hiding this comment.
@BillWagner PS: On your “What’s interesting” – are you using “interesting” as (apocryphally?) used in a Chinese curse? ;-) BTW a certain compiler rejects ... is _ but accepts ... is (_) – can that be justified? Maybe. Is it confusing? Probably. We “live in interesting times”.
There was a problem hiding this comment.
@Nigel-Ecma This comment helped me understand the final changes needed for #1688. ... is _ is a discard_pattern, but ... is (_) is a parenthesised_pattern that encloses a discard_pattern. Because the discard_pattern is a subpattern, it's allowed.
Nigel-Ecma
added
the
meeting: discuss
Co-authored-by: Jon Skeet <skeet@pobox.com>
3 participants
Closes #1602